Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dismiss review feature #12674

Merged
merged 28 commits into from
Feb 11, 2021
Merged

Conversation

a1012112796
Copy link
Member

@a1012112796 a1012112796 commented Sep 1, 2020

@a1012112796 a1012112796 changed the title @a1012112796 Add dismiss review feature Add dismiss review feature Sep 1, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2020

Codecov Report

Merging #12674 into master will increase coverage by 0.03%.
The diff coverage is 31.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12674      +/-   ##
==========================================
+ Coverage   43.29%   43.32%   +0.03%     
==========================================
  Files         646      646              
  Lines       71592    71710     +118     
==========================================
+ Hits        30993    31069      +76     
- Misses      35583    35599      +16     
- Partials     5016     5042      +26     
Impacted Files Coverage Δ
models/branches.go 42.70% <0.00%> (-0.31%) ⬇️
models/issue_comment.go 53.75% <ø> (ø)
models/migrations/migrations.go 4.66% <ø> (ø)
models/migrations/v148.go 0.00% <0.00%> (ø)
models/pull.go 55.17% <0.00%> (ø)
modules/auth/repo_form.go 42.34% <ø> (ø)
modules/graceful/server.go 47.00% <0.00%> (-0.41%) ⬇️
modules/highlight/highlight.go 24.69% <0.00%> (ø)
modules/migrations/gitlab.go 1.04% <0.00%> (-0.10%) ⬇️
modules/session/virtual.go 60.20% <0.00%> (ø)
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fa7a4b...ab6c402. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 1, 2020
@lunny lunny added this to the 1.14.0 milestone Sep 2, 2020
@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 2, 2020
options/locale/locale_en-US.ini Outdated Show resolved Hide resolved
routers/api/v1/api.go Outdated Show resolved Hide resolved
routers/api/v1/repo/pull_review.go Outdated Show resolved Hide resolved
routers/api/v1/repo/pull_review.go Outdated Show resolved Hide resolved
routers/api/v1/repo/pull_review.go Show resolved Hide resolved
routers/routes/routes.go Outdated Show resolved Hide resolved
templates/repo/issue/view_content/pull.tmpl Outdated Show resolved Hide resolved
@bagasme
Copy link
Contributor

bagasme commented Sep 3, 2020

In my case clicking dismiss review button (green X) doesn't dismiss requested review, but instead going back to top of PR page.

@a1012112796
Copy link
Member Author

In my case clicking dismiss review button (green X) doesn't dismiss requested review, but instead going back to top of PR page.

Okay, I will have more checks. Please wait. I'm not good at UI. Thanks.

@bagasme
Copy link
Contributor

bagasme commented Sep 7, 2020

@a1012112796 dismiss worked as expected

Copy link
Contributor

@bagasme bagasme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack: feature worked as expected

@stale
Copy link

stale bot commented Nov 7, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Nov 7, 2020
@lafriks
Copy link
Member

lafriks commented Nov 7, 2020

Please resolve conflicts

@stale stale bot removed the issue/stale label Nov 7, 2020
@lafriks
Copy link
Member

lafriks commented Nov 18, 2020

There is still conflict

@codecov-io
Copy link

codecov-io commented Nov 20, 2020

Codecov Report

Merging #12674 (786f823) into master (45ca2e4) will decrease coverage by 0.00%.
The diff coverage is 53.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12674      +/-   ##
==========================================
- Coverage   42.24%   42.24%   -0.01%     
==========================================
  Files         697      698       +1     
  Lines       76585    76702     +117     
==========================================
+ Hits        32355    32400      +45     
- Misses      38912    38965      +53     
- Partials     5318     5337      +19     
Impacted Files Coverage Δ
models/branches.go 41.61% <0.00%> (-0.29%) ⬇️
models/issue_comment.go 52.71% <ø> (ø)
models/migrations/migrations.go 2.44% <ø> (ø)
models/migrations/v160.go 0.00% <0.00%> (ø)
models/pull.go 55.17% <0.00%> (ø)
modules/auth/repo_form.go 41.59% <ø> (ø)
modules/templates/helper.go 46.89% <0.00%> (-0.19%) ⬇️
routers/repo/pull_review.go 0.00% <0.00%> (ø)
routers/api/v1/repo/pull_review.go 42.95% <31.03%> (-1.26%) ⬇️
modules/notification/mail/mail.go 38.88% <33.33%> (-0.20%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de1e4b2...786f823. Read the comment docs.

models/review.go Outdated Show resolved Hide resolved
@lafriks
Copy link
Member

lafriks commented Jan 24, 2021

Please resolve conflicts

a1012112796 and others added 2 commits January 25, 2021 09:45
* master: (358 commits)
  [skip ci] Updated translations via Crowdin
  Use caddy's certmagic library for extensible/robust ACME handling (go-gitea#14177)
  Redirect on changed user and org name (go-gitea#11649)
  chore: bump minio to RELEASE.2021-01-16T02-19-44Z (go-gitea#14445)
  [skip ci] Updated translations via Crowdin
  CI: skip build steps for cron update works (go-gitea#14443)
  [skip ci] Updated licenses and gitignores
  [skip ci] Updated translations via Crowdin
  just overload to not get it by mistake again ... (go-gitea#14440)
  [skip ci] Updated translations via Crowdin
  Add link to packages in openSUSE build service (go-gitea#14439)
  Improve Description in new/ edit Project template (go-gitea#14429)
  Don't show "Reference in new issue" when issues unit is globally disabled (go-gitea#14437)
  CI: Update license & gitignore by cron (go-gitea#14419)
  Fix close/reopen with comment (go-gitea#14436)
  Add german translation guidelines (go-gitea#14283)
  [skip ci] Updated translations via Crowdin
  Fix lfs preview bug (go-gitea#14428)
  [skip ci] Updated translations via Crowdin
  Bump gsap from 3.5.1 to 3.6.0 (go-gitea#14410)
  ...
@6543
Copy link
Member

6543 commented Jan 26, 2021

@a1012112796 a local git merge --no-ff master && git push should solve this :)

a1012112796 and others added 4 commits January 27, 2021 09:35
* master:
  [skip ci] Updated translations via Crowdin
  Fix bug because of duplicated join (go-gitea#14454)
  Cron job to cleanup hook_task table (go-gitea#13080)
  Fix panic 500 page rendering (go-gitea#14474)
  [skip ci] Updated translations via Crowdin
  Move macaron to chi (go-gitea#14293)
  [skip ci] Updated translations via Crowdin
  Fix incorrect key name so registerManualConfirm setting works as expected. (go-gitea#14455)
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 28, 2021
@6543
Copy link
Member

6543 commented Feb 2, 2021

@a1012112796 would good if you could resolve convlict ;)

* master: (28 commits)
  [Docs] Clone filters (go-gitea#14555)
  update docs to show latest stable version (1.13.2) (go-gitea#14550)
  Adding Chi's GetHead middleware (go-gitea#14541)
  Changelog v1.13.2 (go-gitea#14535) (go-gitea#14543)
  [skip ci] Updated translations via Crowdin
  [API] List, Check, Add & delete endpoints for repository teams (go-gitea#13630)
  [skip ci] Updated translations via Crowdin
  rm redirect (go-gitea#14534)
  Upgrade 'css-minimizer-webpack-plugin' to the latest version (go-gitea#14527)
  Set the name Mapper in migrations (go-gitea#14526)
  Internal ssh server respect Ciphers, MACs and KeyExchanges settings (go-gitea#14523)
  Move middlewares to web/middleware (go-gitea#14480)
  Add Doctor FixWrongUserType (go-gitea#14522)
  [skip ci] Updated translations via Crowdin
  noop (go-gitea#14521)
  Update docs and comments to remove macaron (go-gitea#14491)
  [skip ci] Updated translations via Crowdin
  Fix json charset bug (go-gitea#14514)
  enhancement: add signoff option in commit form (go-gitea#14516)
  Fix load time bug (go-gitea#14508)
  ...
routers/api/v1/repo/pull_review.go Outdated Show resolved Hide resolved
routers/api/v1/repo/pull_review.go Outdated Show resolved Hide resolved
routers/api/v1/repo/pull_review.go Outdated Show resolved Hide resolved
services/pull/review.go Outdated Show resolved Hide resolved
a1012112796 and others added 4 commits February 9, 2021 09:53
Co-authored-by: zeripath <art27@cantab.net>
* master: (22 commits)
  Add support for ref parameter to get raw file API (go-gitea#14602)
  Fixed irritating error message related to go version (go-gitea#14611)
  Use OldRef instead of CommitSHA for DeleteBranch comments (go-gitea#14604)
  Add information on how to build statically (go-gitea#14594)
  [skip ci] Updated translations via Crowdin
  Exclude the current dump file from the dump (go-gitea#14606)
  Remove spurious DataAsync Error logging (go-gitea#14599)
  [API] Add  delete release by tag & fix unreleased inconsistency (go-gitea#14563)
  Fix rate limit bug when downloading assets on migrating from github (go-gitea#14564)
  [API] Add affected files of commits to commit struct (go-gitea#14579)
  [skip ci] Updated licenses and gitignores
  Fix locale init (go-gitea#14582)
  Add Content-Length header to HEAD requests (go-gitea#14542)
  Honor REGISTER_MANUAL_CONFIRM when doing openid registration (go-gitea#14548)
  Fix lfs file viewer (go-gitea#14568)
  Fix typo in generate-emoji.go (go-gitea#14570)
  Fix bug about ListOptions and stars/watchers pagnation (go-gitea#14556)
  Fix gpg key deletion (go-gitea#14561)
  [API] GetRelease by tag only return release (go-gitea#14397)
  Reduce data races (go-gitea#14549)
  ...
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 10, 2021
@6543
Copy link
Member

6543 commented Feb 11, 2021

🚀

@6543 6543 merged commit ac70163 into go-gitea:master Feb 11, 2021
@a1012112796 a1012112796 deleted the feature/dismissing_reviews branch February 12, 2021 02:51
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants